Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libgetopts: tweak HasHarg/Occur docs #16252

Merged
merged 1 commit into from Aug 6, 2014
Merged

libgetopts: tweak HasHarg/Occur docs #16252

merged 1 commit into from Aug 6, 2014

Conversation

conradkleinespel
Copy link
Contributor

@conradkleinespel conradkleinespel commented Aug 4, 2014

I found the current docs to be a little confusing. I believe this makes them straight to the point and less confusing.

Optional,
/// The option occurs once or multiple times.
/// The option occurs zero or more times.
Copy link
Member

@alexcrichton alexcrichton Aug 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be one or more?

Copy link
Contributor Author

@conradkleinespel conradkleinespel Aug 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it shouldn't. That's partly why I'm making this PR. With Occur set to Multi, it is allowed for the option not to show up at all. With this PR, I'm assuming that the code is correct but the docs a little outdated.

@alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Aug 4, 2014

Ok, thanks! Could you squash the commits together as well?

@conradkleinespel
Copy link
Contributor Author

@conradkleinespel conradkleinespel commented Aug 4, 2014

Done.

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging conradkleinespel/rust/master = c74d320 into auto

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conradkleinespel/rust/master = c74d320 merged ok, testing candidate = 2a47fa7

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 2a47fa7

bors added a commit that referenced this issue Aug 6, 2014
I found the current docs to be a little confusing. I believe this makes them straight to the point and less confusing.
@bors bors closed this Aug 6, 2014
@bors bors merged commit c74d320 into rust-lang:master Aug 6, 2014
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants